Move "ignore unknown parameters" in the common section between GET and POST#384
Move "ignore unknown parameters" in the common section between GET and POST#384martinbonnin wants to merge 1 commit intomainfrom
Conversation
benjie
left a comment
There was a problem hiding this comment.
Interesting. I think this is going to be more problematic for GraphQL GET queries where ?api_token=, ?csrf_token=, and similar fields are more likely to occur. These will typically need to be handled at the middleware level (before GraphQL), so it would normally not be suitable to encode them as JSON inside extensions for example. I think this one needs more thought/discussion. People should be using headers for these, but it's not always that easy.
|
Another type of query parameter is those involved in routing, e.g. |
|
Personally I think this paragraph has no force. If someone needs a custom field for their own needs they will add it however they choose to do so, since it will need to be done on both the server and client ends. I have an implementation of persisted documents, for instance, but it could be auth related or something else. Can’t use regular tooling since it’s nonstandard anyway. So I don’t really see the purpose of using MUST here. The only consequence that can occur is that tooling might not support custom fields due to the word “MUST” which just makes it harder on the developer that needs a custom field. Perhaps this forces them to use an alternate implementation method like headers but honestly a compliant request won’t work without the additional header so what’s the difference? You can also argue that since it’s not going to work anyway, use of MUST is appropriate since the request won’t process and therefore is non-standard by definition. |
|
The benefit to not-specifying “MUST” is simply that tooling might better support custom requirements by a developer. Whether it’s inspection, server or client libraries. As an example, I know GraphQL js totally rejects an introspection response listing a nonstandard reserved field. Completely breaks GraphiQL if you add one (e.g. Rather not repeat that decision here. |
Indeed. Sounds selfish of GraphQL to claim the whole query string. But there's no silver bullet, right? Either we let it go (and risk nameclashes like for directive names) or we claim a prefix (i.e. I'm fine letting this one go. The probability of clash is probably low ( |
This is not specific to JSON